Skip to content

feat(cli): port supabase stop and status commands to native TypeScript#5765

Merged
Coly010 merged 99 commits into
developfrom
cli/port-stop-status-commands
Jul 8, 2026
Merged

feat(cli): port supabase stop and status commands to native TypeScript#5765
Coly010 merged 99 commits into
developfrom
cli/port-stop-status-commands

Conversation

@Coly010

@Coly010 Coly010 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Ports supabase stop and supabase status from Go-proxy stubs to native TypeScript in the legacy CLI shell (CLI-1324).

  • Both commands now talk directly to Docker/Podman via subprocess, replicating Go's label-filtering and container-naming scheme byte-for-byte. Legacy start is still Go-proxied, so this intentionally does not route through @supabase/stack/effect's daemon-based orchestration model — that substrate manages a different set of containers than the ones Go's binary actually creates, and using it would silently no-op against a real running stack.
  • New shared infrastructure (legacy-docker-lifecycle, legacy-go-jwt, legacy-local-config-values, legacy-api-url) is reused by both commands, matching Go's local-dev defaults exactly — including a Go-byte-exact JWT signer, since @supabase/stack's own JWT generator uses a different issuer/claim order than what Go prints for local dev keys.
  • Adds *.live.test.ts as a documented test category (AGENTS.md) alongside unit/integration/e2e: black-box subprocess tests run by the cli-e2e-ci harness against a real platform. stop/status don't call the Management API, so their live tests spin up a real local Docker stack instead and verify against it directly (e.g. confirming Docker itself has no containers left after stop, not just trusting the CLI's exit code).

Notable review findings fixed along the way

  • Table/status output was colorizing based on stderr's TTY status while writing to stdout — piping stdout while stderr stayed a TTY (supabase status | less) would have corrupted output with ANSI escapes (the same bug class CLI-1546 fixed once before).
  • --override-name was leaking into pretty-mode output; Go's PrettyPrint rebuilds a fresh, un-overridden view and ignores it there.
  • --backup/--no-backup: Go's --backup flag is dead code (declared, never bound to a variable in cmd/stop.go) — the port now matches that exactly instead of an intended-but-never-true semantic.
  • Docker/Podman-both-missing errors now name the actual root cause instead of a generic "failed to ..." string.

Post-review hardening: scoping and consolidation

Two structural fixes on top of the port, addressing drift introduced while iterating on review feedback:

  • next/ no longer inherits Go-parity config semantics. The four viper-compat behaviors added during review (unconditional [remotes.*] project-id checks, the deprecated-provider WARN, the widened env() reference pattern, and comma-split coercion into array-typed fields) are now gated behind an opt-in goViperCompat flag on LoadProjectConfigOptions — default off restores the pre-review behavior for next/, packages/stack, and other non-parity consumers, following the same opt-in pattern as tomlOnly/skipEnvLocal. Only legacy-shell callers opt in. Regression tests pin the default-off behavior, including the next start config-load path that would otherwise have started hard-failing on a malformed [remotes.*].project_id.
  • Go's Config.Validate now has exactly one TS home. legacy/shared/legacy-config-validate.ts replaces the validation orchestration previously duplicated between legacy-db-config.toml-read.ts (db/migration family) and legacy-local-config-values.ts (status/stop); both callers build a normalized LegacyConfigValidationInput from their own pipelines and call the shared validator, and a cross-caller parity test feeds identical broken configs through both real pipelines asserting identical error strings. Consolidating surfaced and fixed three real divergences between the two copies: db.major_version = 0 now emits Go's Missing required field in config: db.major_version (the db loader previously emitted a non-Go message), and the captcha provider-enum and email content-vs-content_path checks are now present in both paths.

CLOSES CLI-1324

Coly010 added 2 commits July 2, 2026 11:11
Replaces the Go-proxy stubs for `stop`/`status` with native Effect
implementations that talk directly to Docker/Podman via subprocess,
replicating Go's label-filtering and container-naming scheme byte-for-byte.
Legacy `start` is still Go-proxied, so this intentionally does not go
through `@supabase/stack/effect`'s daemon-based orchestration model — that
substrate is incompatible with the real containers Go's binary creates.

Adds shared Docker/config infrastructure (`legacy-docker-lifecycle`,
`legacy-go-jwt`, `legacy-local-config-values`, `legacy-api-url`) used by
both commands, and fixes several correctness issues found during review:
stdout colorization keyed off the wrong stream's TTY status, `--override-name`
incorrectly leaking into pretty-mode output (Go ignores it there), and the
`--backup`/`--no-backup` formula not matching Go's actual (dead-flag)
behavior.

Fixes CLI-1324
…e.test.ts

Documents the *.live.test.ts convention in AGENTS.md (a 4th test category
alongside unit/integration/e2e): black-box CLI subprocess tests executed by
the cli-e2e-ci harness against a real supabox stack. Clarifies how
local-dev-stack commands like stop/status fit this pattern despite never
calling the Management API — they only need the real Docker daemon the
cli-e2e-ci runner also provides, so they reuse the existing describeLive
gate rather than a dedicated one.

Adds stop.live.test.ts and status.live.test.ts, each spinning up a real
local Docker stack (init -> start, excluding the heaviest services) in an
isolated temp directory and verifying the command under test against the
real containers, with best-effort cleanup on every path.
@Coly010 Coly010 self-assigned this Jul 2, 2026
@Coly010

Coly010 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

noBackup: Flag.boolean("no-backup").pipe(

P2 Badge Avoid colliding with backup's negated flag

Effect boolean flags register both the positive and negated token, so Flag.boolean("backup") above already claims --no-backup as the false form of the hidden backup flag. Defining a separate noBackup boolean on the same token can route supabase stop --no-backup to backup=false or otherwise collide during parsing, leaving flags.noBackup false and skipping the volume-prune path even though the user requested data deletion; model the hidden --backup=false flag without generating a --no-backup alias.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/status/status.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.values.ts Outdated
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
@Coly010

Coly010 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this — I dug into it and don't think there's a collision here, though the concern was reasonable to raise given how the two flags look side by side.

Effect's flag registry (effect/unstable/cli/internal/parser.ts:232-252, current pinned version 4.0.0-beta.87) only indexes each flag's literal declared name (plus Flag.withAlias entries) at build time — it does not insert a synthetic no-<name> key for boolean flags. The --no-<name> negation is resolved lazily per-token in resolveFlag (parser.ts:348-380): it tries an exact-name match first (parser.ts:353-354) and only falls back to stripping the no- prefix and negating (parser.ts:361-369) when there's no direct match. Since noBackup is declared as Flag.boolean("no-backup"), its literal name "no-backup" is the direct match, so any --no-backup token resolves straight to noBackup — the hidden backup flag's negation path is never consulted, and there's no registry-level duplicate-name conflict either (backup and no-backup are distinct literal strings).

I confirmed this empirically by running the exact flag config through Command.runWith:

["--no-backup"]                 → { backup: true,  noBackup: true,  all: false }
[]                               → { backup: true,  noBackup: false, all: false }
["--backup=false"]              → { backup: false, noBackup: false, all: false }
["--no-backup", "--backup"]     → { backup: true,  noBackup: true,  all: false }

noBackup comes through correctly in every combination, and stop.handler.ts reads flags.noBackup (not a derived backup value) to drive the volume-prune path, so it isn't affected either way.

This also matches the Go CLI's own structure: cmd/stop.go:26-29 declares --backup as a hidden flag that's never actually read in RunE (Go's version is dead too) and --no-backup as a completely independent BoolVar. The two separate Flag.boolean declarations here are a deliberate 1:1 mirror of that, not an accidental duplication. Given that, I'll leave the flag declarations as-is, but appreciate you taking a close look at the flag-parsing semantics.

Coly010 added 3 commits July 2, 2026 12:13
…5765)

Three accepted findings from PR review, each verified against Go source
before implementing:

- Sanitize the config/env-derived project id before building the Docker
  label filter in both `stop` and `status`. Go's `Config.Validate`
  sanitizes `Config.ProjectId` once at config-load time
  (pkg/config/config.go:938-944), and every later reader — including the
  Docker label `start` writes (internal/utils/docker.go:375) — sees that
  same sanitized string. Without this, a dirty `project_id` (spaces,
  leading punctuation) would filter on a value `start` never labeled,
  silently matching nothing. The explicit `--project-id` bypass on `stop`
  stays raw, matching Go's stop.go:19-20.

- `status --override-name` with an unrecognized field key is now silently
  ignored instead of a hard error, matching Go's `go-env` Unmarshal, which
  never checks its input map for unmatched keys (verified against the
  vendored go-env@v0.1.2 source).

- `status` no longer hard-fails when `supabase/config.toml` is absent;
  Go's `flags.LoadConfig` treats a missing file as a no-op and proceeds
  with template defaults (pkg/config/config.go:655-656), so this now
  decodes an empty document through the shared config schema for its
  defaults instead of erroring. (The broader gap — Go's automatic
  `SUPABASE_<FIELD>` env-var binding via viper, which `@supabase/config`
  doesn't have an equivalent for — is a larger, cross-cutting
  `@supabase/config` feature affecting every ported command, called out
  as a follow-up rather than folded into this fix.)

Two other findings were investigated and rejected with cited evidence
(directly on the PR): a claimed `--backup`/`--no-backup` flag collision
(empirically disproven against Effect's parser), and image-name-based
`--exclude` matching (the underlying config.toml schema has no field to
check, on either CLI).
…0O86N3Lyo)

Go's status.toValues() gates each service on `--exclude` matching either
the container id or the Docker image's short name (ShortContainerImageName).
The TS port only checked container ids. Port the short-name extraction and
check it against the same default images the embedded Dockerfile manifest
already provides, since the relevant Go config fields (KongImage, Image,
etc.) all carry `toml:"-"` and are never user-overridable.
…us-commands

# Conflicts:
#	apps/cli/docs/go-cli-porting-status.md
@Coly010

Coly010 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b42381e79

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b42381e79

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Coly010 added 8 commits July 2, 2026 13:25
…OErm0O86N4wLX)

Go's stop.Run checks len(projectId) > 0 (internal/stop/stop.go:18), not just
whether --project-id was set, so an explicit but empty value falls through
to config.toml resolution. The TS port only checked Option.isSome, so
`--project-id ""` resolved to the bare all-projects label filter instead.
…6N4wLa, PRRT_kwDOErm0O86N4wLu)

Go registers both --override-name and --exclude as pflag StringSliceVar
(cmd/status.go:36-37), which CSV-splits each occurrence and accumulates
across repeats. The TS flags only handled repetition, so a single
comma-separated value like `--exclude kong,auth` produced one malformed
entry instead of two. Reuses the shared legacyParseStringSliceFlag already
applied to sso/postgres-config for the same StringSlice parity gap.
…6N4wL1)

Go's Config.Validate fails config-load when auth.jwt_secret is set but
shorter than 16 characters (pkg/config/apikeys.go:45-47), before any command
can render output. The TS resolver accepted any non-empty value and signed
ANON_KEY/SERVICE_ROLE_KEY with it, letting `status -o env/json` succeed and
print keys for a config the Go CLI and local stack both reject.
… PRRT_kwDOErm0O86N4wLx)

Go's config loader binds Viper with SetEnvPrefix("SUPABASE") + AutomaticEnv()
(pkg/config/config.go:529-535), so SUPABASE_AUTH_JWT_SECRET/PUBLISHABLE_KEY/
SECRET_KEY/ANON_KEY/SERVICE_ROLE_KEY override the corresponding config.toml
value at higher precedence. legacyResolveLocalConfigValues only read the
decoded config object, so a local stack started with those env overrides
had `status` print keys that didn't match the running Auth service. Scoped
to exactly the 5 auth fields this module reads, not a general
@supabase/config port of Viper's AutomaticEnv.
…ed (review: PRRT_kwDOErm0O86N4wLk)

Go's generateJWT signs anon/service_role with the first key in
auth.signing_keys_path (RS256/ES256) instead of HMAC when that file
resolves to a non-empty JWK array (pkg/config/apikeys.go:76-113). The TS
resolver only ever HMAC-signed with jwt_secret. Ports GenerateAsymmetricJWT
via legacyGenerateAsymmetricGoJwt (RFC 7517 JWK -> Node crypto private key,
ieee-p1363 signature encoding for ES256's raw r||s format) and wires it
into legacyResolveLocalConfigValues, which now needs workdir to resolve a
relative signing_keys_path against <workdir>/supabase.
Go's jwkToRSAPrivateKey/jwkToECDSAPrivateKey reject a JWK whose kty doesn't
match its claimed alg, and an EC key whose curve isn't P-256
(pkg/config/auth.go). legacyGenerateAsymmetricGoJwt only checked alg, so an
EC key forged with alg: RS256 (or a non-P-256 curve claimed as ES256)
signed "successfully" and produced a spec-invalid token that silently
fails verification instead of raising an error. Found independently by the
security and DX reviewers in review-changes.
…tusValues twice

status.handler.ts called legacyStatusValues twice per invocation (real
values, then pretty-mode values with an empty override map) — only the
first call was wrapped in Effect.try. With signing_keys_path support this
doubled file I/O + JWT signing on every text-mode run, and left the second
call able to throw an uncaught exception if it ever diverged from the
first. Splits legacyStatusValues into legacyResolveStatusState (the
throwing half: local config resolution + gating) and
legacyStatusValuesFromState (pure name remapping), so the handler resolves
state once, guards it once, and reuses it for both value maps.
…ration coverage

Go's Config.Validate fails a bad auth.signing_keys_path with "failed to
read signing keys: %w" (open failure) or "failed to decode signing keys:
%w" (parse failure) (pkg/config/config.go:1059-1062). The TS port let
readFileSync/JSON.parse's raw Node error text through unwrapped instead.
Also adds status.integration.test.ts coverage for the SUPABASE_AUTH_* env
override and asymmetric-signing-key behaviors, which previously only had
unit-level coverage on the pure resolver.
@Coly010 Coly010 marked this pull request as ready for review July 2, 2026 14:06
@Coly010 Coly010 requested a review from a team as a code owner July 2, 2026 14:06
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@f5fc156a18b77526db855ec0d29804d6bcbd319e

Preview package for commit f5fc156.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60877ba497

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Coly010 added 5 commits July 2, 2026 15:49
…spection (ci: e2e shard 1/3)

Go's assertContainerHealthy never special-cases a missing container — it
wraps whatever ContainerInspect returns, so the real daemon error text
("No such container: ...") flows through. The TS port collapsed that case
into a hardcoded "no such container" string via an "absent" sentinel,
discarding the real text.

Separately, legacyInspectContainerState's Effect.all([exitCode, stdout,
stderr]) ran sequentially by default, awaiting exitCode (Node's "exit"
event) before ever subscribing to the stdout/stderr streams. Node's "exit"
can fire before a fast process's stdio pipes are drained, so the real
Docker CLI's stderr was silently lost in the real subprocess environment
even after removing the hardcoded string — reproduced against a real
docker CLI subprocess, confirmed via runParity's stderr comparison in
apps/cli-e2e. Same fix applied to the two other call sites in this file
that share the pattern (legacyListContainersByLabel, legacyListVolumesByLabel).
…e2e shard 1/3)

Go prints this line via an unconditional, immediate fmt.Fprintln before any
Docker call runs (docker.go:97), routed straight to stdout in non-interactive
mode (tea.go's fakeProgram). The TS port gated it behind the shared
output.task spinner's 200ms debounce, so the line was silently dropped
whenever the underlying Docker calls resolved faster than that threshold —
exactly what happens against the mocked/replayed Docker CLI in the e2e
harness. Printing it directly via output.raw removes the race entirely.
…ity request logs (ci: e2e shard 1/3)

stop/status parity comparisons were failing on request-log mismatches that
reflect nothing about CLI behavior: the real docker CLI issues a fresh
HEAD /_ping handshake before every subprocess invocation (Go's SDK pings
once per command via a persistent client), and negotiates its own API
version segment into the URL path (/v1.51/ vs /v1.53/) based on the
installed docker CLI/daemon, not anything the command controls. Strip both
before comparing so parity reflects the actual Docker operations performed
rather than client plumbing, mirroring the equivalent normalization already
used for fixture matching in apps/cli-e2e/src/server/placeholder.ts.
…s (review: PRRT_kwDOErm0O86N7ctR)

Go's Viper binds SetEnvPrefix("SUPABASE") + AutomaticEnv() over every
config field via UnmarshalExact's struct walk (pkg/config/config.go:531-535,
698-705), including the plain-string Auth.SigningKeysPath field
(pkg/config/auth.go:164) — not just the 5 auth fields this module already
wrapped with envOverride. Load() resolves that override before Validate()
opens and parses the JWK file (config.go:735-745, 1059-1062), so an
env-only SUPABASE_AUTH_SIGNING_KEYS_PATH was silently ignored and status
fell back to HMAC-signed keys the running Auth service would reject.
… status (review: PRRT_kwDOErm0O86N7ctY)

Go's Config.Load runs loadNestedEnv (supabase/.env and .env.local via
godotenv.Load, which never overrides an already-set var) before
loadFromFile wires up Viper's AutomaticEnv (pkg/config/config.go:735-745,
528-535) — so an env-file-only SUPABASE_PROJECT_ID overrides config.toml's
project_id too, not just an ambient shell export. Both handlers only read
process.env directly, missing that middle step: an env-named stack could be
left running by stop, or status could target the wrong container.

@supabase/config's loadProjectEnvironment already implements the same
"ambient wins over .env/.env.local" layering (used today only for env()
interpolation inside config.toml), so both handlers now resolve
SUPABASE_PROJECT_ID through it instead of process.env directly, and pass
the same resolved environment into loadProjectConfig so the .env files
aren't parsed twice. Falls back to process.env directly when no
supabase/config.toml exists anywhere (loadProjectEnvironment resolves to
null in that case), preserving the existing ambient-only behavior for that
scenario.

The identical gap in LegacyCliConfig.projectId (used by several other
already-ported commands) is out of scope here and left for a follow-up.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb101635cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts Outdated
Coly010 added 3 commits July 7, 2026 15:48
…us-commands

# Conflicts:
#	apps/cli/docs/go-cli-porting-status.md
#	apps/cli/src/legacy/commands/seed/buckets/buckets.handler.ts
…ets hoist merge

develop's db push/reset/start port (#5715) hoisted the seed-buckets
config load out of buckets.handler.ts and into the shared
legacySeedBucketsRun (also used by db reset --local), which
superseded the inline goViperCompat opt-in added on this branch.
Carry the opt-in over to the new call site so seed buckets and
db reset --local keep Go-parity config semantics.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ddbeb555a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread packages/config/src/io.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
Coly010 added 5 commits July 8, 2026 10:28
…us-commands

# Conflicts:
#	apps/cli/src/legacy/commands/secrets/set/set.handler.ts
#	packages/config/src/io.ts
…e quality)

Collapses a .pipe() call back to oxfmt's canonical single-line form;
introduced by the develop merge conflict resolution and left unformatted.
…ting scheme (review: #PRRT_kwDOErm0O86OsZMu)

Go's hookConfig.validate calls url.Parse before the scheme switch
(pkg/config/config.go:1497-1499) and rejects a malformed URI (e.g. an
unterminated IPv6 host like http://[::1) before status/stop reach Docker.
The TS validator only regex-extracted a scheme prefix, so any http:/https:
prefix was accepted regardless of the rest of the URI. Reuses the existing
legacyGoUrlParse port (already used for studio.api_url) instead of the
scheme regex.
…PI keys (review: #PRRT_kwDOErm0O86OrzAU)

Go's config.Secret fields (auth.jwt_secret/publishable_key/secret_key/
anon_key/service_role_key, pkg/config/auth.go:181-185) are decrypted by
DecryptSecretHookFunc unconditionally during UnmarshalExact
(pkg/config/secret.go:30-73), failing config load on an undecryptable
value before status/stop continue. @supabase/config's schema only tags
these fields for later Redacted wrapping and never decrypts them, so the
status/stop resolver was using an "encrypted:..." value as literal (wrong)
key material, and never failed on an undecryptable one. Reuses the
existing eciesjs-based decrypt primitives from legacy-vault-decrypt.ts
(already used by the db/migration config loader for the same Go hook).
…ection (review: #PRRT_kwDOErm0O86Ossk3)

Go's -o/--output is a root PersistentFlags() enum (cmd/root.go:330) every
subcommand inherits, so "stop -o csv"/"-o table" is rejected by pflag at
parse time even though stop.RunE never reads the value itself. The doc
claimed Go has no such flag and that TS accepting an unknown value was a
harmless divergence — in fact stop.command.ts already rejects the same
values via withLegacyCommandInstrumentation's default outputFormats gate,
matching Go exactly. No behavior change, just correcting the doc.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c6d766f35

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/secrets/set/set.handler.ts
Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts
Comment thread packages/config/src/io.ts
Coly010 added 4 commits July 8, 2026 11:55
…alidation

Go's Viper AutomaticEnv (config.go:581-586) lets SUPABASE_AUTH_HOOK_*/
SUPABASE_AUTH_CAPTCHA_*/SUPABASE_AUTH_PASSKEY_*/SUPABASE_AUTH_WEBAUTHN_*/
SUPABASE_AUTH_EMAIL_SMTP_* override any already-declared config.toml section
before Config.Validate runs. legacy-local-config-values.ts only read the
TOML-decoded (or raw-document) values for these sections, so an env-only
override that should surface a missing uri/provider/secret/host/etc. was
silently ignored by the native status/stop port. Each override is gated on
the raw document section already being present, matching AutomaticEnv's own
"key must already be in the merged config" behaviour empirically verified
against apps/cli-go/pkg/config.

(review: #PRRT_kwDOErm0O86O-GCN, #PRRT_kwDOErm0O86O-GCS, #PRRT_kwDOErm0O86O-GCW, #PRRT_kwDOErm0O86O-GCb)
…: #PRRT_kwDOErm0O86PMyg7)

Go's flags.LoadConfig swallows any Config.Load error non-fatally
(internal/secrets/set/set.go:22-24), including the invalid-format error
Config.Validate raises for a bad remotes.*.project_id
(pkg/config/config.go:996-1001). The TS handler only recovered from
DuplicateRemoteProjectIdError, so a malformed remote block could abort an
otherwise-valid secrets set; now InvalidRemoteProjectIdError is handled the
same way.
…review: #PRRT_kwDOErm0O86OsZM5)

Go's external.validate() (pkg/config/config.go:1419-1451) validates every
enabled auth.external provider, including arbitrary/unmodeled names, since
Go's External field is a genuine map[string]provider. @supabase/config's
schema only models ~20 known providers and silently drops anything else at
decode time, so status/stop (which decode through that schema) could accept
a misconfigured unmodeled provider Go rejects. auth.external was already
D-only (legacy-db-config.toml-read.ts) per legacy-config-validate.ts's
module header; this ports the identical inline raw-document check to L
(legacy-local-config-values.ts), run after the shared
legacyValidateResolvedConfig call per the documented sms/external-vs-
third_party ordering tradeoff.
…l providers (review: #PRRT_kwDOErm0O86OsslE)

Go's (s *sms) validate() (apps/cli-go/pkg/config/config.go:1348-1410) is a
boolean switch that validates ONLY the first enabled provider in a fixed
priority order (twilio, twilio_verify, messagebird, textlocal, vonage) — a
later enabled-but-incomplete provider is never inspected. The sms schema
instead attached an independent requiredWhenEnabled check to each provider
sub-struct, validating every enabled provider at decode time, so a stale
secondary [auth.sms.*] block Go silently ignores could make native
status/stop reject a config Go accepts. Replaced the five per-provider
checks with a single struct-level check implementing Go's switch-priority
semantics.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cac1e5f62c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/status/status.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
… (review: #PRRT_kwDOErm0O86OsZMx)

Go's Config.Validate only ever checks remotes.*.project_id format for every
remote block (apps/cli-go/pkg/config/config.go:996-1001, "Since remote
config is merged to base, we only need to validate the project_id field").
Every other business-rule check (Auth.External.validate(), Auth.Sms.validate(),
etc.) runs exactly once, against the merged effective config
(config.go:1136-1152), and is never iterated over c.Remotes[*].

ProjectConfigSchema's remotes field reused the same auth/db/etc. schema
pieces as the top level, and those embed .check()-based business-rule
refinements directly, so a non-selected [remotes.*] block (the common case
for status/stop, which run without a projectRef) was fully struct- and
business-rule-validated during decode. A remote stub like
[remotes.prod.auth.external.github] enabled = true with no secret would fail
the whole config load even though Go itself never looks at that remote's
business rules unless it's the one merged into effect.

Extracts remotes into an exported RemotesSchema and decodes it separately
with Effect Schema's disableChecks: true, which keeps full type/shape
decoding (so a genuinely malformed value still fails, matching Go's
unconditional UnmarshalExact struct decode) while skipping the merged-
config-only business rules for every remote, selected or not. The rest of
the document keeps decoding with checks on, so a SELECTED remote (merged
into the effective config by applyRemoteOverride) is still fully validated.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b69797ad0b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/commands/stop/stop.handler.ts
Comment thread packages/config/src/io.ts
Coly010 added 2 commits July 8, 2026 13:21
…: #PRRT_kwDOErm0O86POG5n)

Go's Auth.Email.Template/Notification are genuine map[string]emailTemplate/
map[string]notification (apps/cli-go/pkg/config/auth.go:247-248) with no key
restriction — (e *email) validate(fsys) iterates every entry regardless of
name. The email schema instead restricted record keys to a hard-coded
template/notification name pattern, so an unrecognized block like
[auth.email.template.custom] failed schema decode entirely, making native
status/stop (and any other @supabase/config consumer) reject a config Go
accepts. Relaxed both record key schemas to accept any string, matching Go's
open map.
… gaps in status/stop config resolution

Four related Go-parity gaps in legacyResolveLocalConfigValues, all variations
on values not being re-checked against the post-env-override state before
validation runs (continuing the pattern fixed in 08ed337 for hook/captcha/
passkey/smtp):

- project_id: Go's mergeDefaultValues merges sanitizeProjectId(filepath.Base(
  cwd)) in as a viper default BEFORE config.toml is merged (config.go:690-699,
  via Eject), so c.ProjectId is never Go's zero value by the time Validate
  runs. A workdir whose basename sanitizes to "" (e.g. "!!!") therefore fails
  config loading in Go even with no project_id key in the file at all; this
  resolver only used the raw (unsanitized, non-defaulted) config value,
  silently passing `undefined` through and skipping the check entirely.

- auth.email.notification/template: SUPABASE_AUTH_EMAIL_NOTIFICATION_<NAME>_
  ENABLED/_CONTENT_PATH and SUPABASE_AUTH_EMAIL_TEMPLATE_<NAME>_CONTENT_PATH
  overrides now apply before deciding whether/what content_path to validate,
  instead of only reading the schema-decoded (pre-override) value.

- auth.sms: @supabase/config's sms schema implements Go's provider-switch
  validation (cac1e5f) but only at decode time, before this shell's env
  overrides are resolved. Added validateAuthSmsProviders, which re-runs the
  same fixed-priority switch against the raw document with
  SUPABASE_AUTH_SMS_<PROVIDER>_* overrides applied, so an env-only enabled
  provider is still checked.

- auth.external: SUPABASE_AUTH_EXTERNAL_<NAME>_ENABLED/_CLIENT_ID/_SECRET
  overrides now apply before validateAuthExternalProviders' required-field
  checks, covering known and unmodeled provider names alike.

(review: #PRRT_kwDOErm0O86POG5r, #PRRT_kwDOErm0O86POG5w, #PRRT_kwDOErm0O86POG5z, #PRRT_kwDOErm0O86POG54)
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
Comment thread packages/config/src/auth/providers.ts
Coly010 added 2 commits July 8, 2026 15:53
… (review: #PRRT_kwDOErm0O86PQqpC)

`packages/config/src/auth/providers.ts` renamed the `slack` provider key to
`slack_oidc` (matching Go's deprecated-provider handling), but the generated
docs asset at `apps/docs/public/cli/config.schema.json` was never
regenerated, leaving it stale. Ran `bun scripts/generate-docs.ts` from
`apps/cli` (the same command `apps/docs`'s own `generate`/`build` scripts
invoke) to bring it back in sync with `ProjectConfigSchema`.
…atus/stop validation (review: #PRRT_kwDOErm0O86PQqa2)

Go's Viper setup (`ExperimentalBindStruct` + `SetEnvPrefix("SUPABASE")` +
`AutomaticEnv()`, `pkg/config/config.go:580-586`) binds every leaf field of
the config struct to a `SUPABASE_*` env override before `Config.Validate`
runs. `Auth.MFA` and `Auth.ThirdParty`'s provider structs are value-typed
(never `nil`, `auth.go:191-198,317-320`), so — unlike `Auth.Hook`/smtp's
pointer-typed fields, which need TOML-presence gating — these overrides
apply unconditionally, even when the section is absent from both the file
and the default template (e.g. `[auth.mfa.web_authn]` and
`[auth.third_party.workos]`).

`legacyResolveLocalConfigValues` read `config.auth.mfa.*` and
`config.auth.third_party.*` straight off the decoded TOML with no env
override, so e.g. `SUPABASE_AUTH_MFA_TOTP_ENROLL_ENABLED=true` or
`SUPABASE_AUTH_THIRD_PARTY_FIREBASE_ENABLED=true` could change Go's
validation outcome while this port silently ignored them. Wires both blocks
through `legacyEnvOverrideBool`/`envOverride`, matching the existing
hook/smtp precedent in the same file.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5fc156a18

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
Comment thread apps/cli/src/legacy/shared/legacy-config-validate.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
@Coly010 Coly010 enabled auto-merge July 8, 2026 15:28
@Coly010 Coly010 added this pull request to the merge queue Jul 8, 2026
Merged via the queue into develop with commit 02201c9 Jul 8, 2026
41 checks passed
@Coly010 Coly010 deleted the cli/port-stop-status-commands branch July 8, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants